fix refund bugs - #5

レビューしました。結論として race condition の修正そのものは妥当 です。

SELECT … FROM transactions WHERE id = $1 FOR UPDATE によって、同一 payment に対する並行 restore() が READ COMMITTED 下でも正しく直列化されます（ロック取得後の exists / already の読み取りは新しいスナップショットを見るため）。テストが狙っている二重返金は実際に塞がっています。復元プランを INSERT … ON CONFLICT DO NOTHING より前に移す変更も、このロックがある前提では挙動中立です。デッドロック環も増えていません（pay() は merchants→buckets、expire_due は SKIP LOCKED、payment の transaction 行より先に bucket をロックする経路はありません）。

その上で 3 点コメントします。1 と 2 は実装の指摘、3 はドキュメントです。

1. 有効期限ガードが bucket をロックしていない（crates/melon-db/src/ops.rs）

復元プランを組む際の SELECT は tb.expires_at をロックなしで読んでいます。

SELECT le.bucket_id, le.amount, tb.expires_at FROM ledger_entries le
  JOIN topup_buckets tb ON tb.id = le.bucket_id
 WHERE le.transaction_id = $1 AND le.kind = 'payment' ORDER BY le.seq

このチェックと、後段の

UPDATE topup_buckets SET remaining_amount = remaining_amount + $1, status = 'active' WHERE id = $2

の間に並行する expire_due() のスイープが入り込むと、失効直後の bucket を status = 'active' で復活させてしまいます。結果として breakage の二重計上が起こり得ます。本 PR が閉じようとしているのと同じ TOCTOU の形です。

SELECT に FOR UPDATE OF tb を付けて bucket 行ごとロックしてください。pay() が有効期限の早い順に消費する以上、le.seq 順のロック順序は payment 間で一貫するので、追加のデッドロック要因にはならないはずです。

なお下の 2 を採用する場合、「有効か失効か」で処理を分岐させることになるため、判定根拠である expires_at をロックして読むことは必須になります。1 と 2 は独立ではなく、セットで直す形になります。

2. void() を失効済みバケットでも成功させたい

void() も同じ restore() を通るため、寄与バケットが 1 つでも失効した時点で、誤請求に対する技術的リバーサルが恒久的に不可能になります。返金の拒否は妥当ですが、void は技術的訂正なので常に成功できる必要があると考えます。

そこで refund は現状の 422 拒否のまま、void のみ「復元して即失効」させる 形を提案します。restore() に kind 由来のポリシーを渡し、reversal かつバケットが失効済みの場合だけ次のように書きます。

• status = 'active' への書き戻しはしない（バケットは失効のまま）
• reversal ポスティング +restore_amt は通常どおり記帳する（取引の打ち消しが台帳上で完結する）
• 同時に expiry ポスティング -restore_amt を記帳し、remaining_amount を 0 に戻す

会計的にはこれが実態と一致します。その支払いが最初から無かったとしたら、その価値はバケットに残ったまま期限到来で失効益（breakage）になっていたはずなので、void 後に breakage へ計上するのが正しい姿です。二重計上にもなりません — expire_due() は status = 'active' の行しか拾わないので、二度目のスイープは来ません。利用者から見た復活残高はゼロで、これも正しい挙動です。

スキーマ変更は不要です。0001_init.sql の CHECK は expiry の transaction_id NULL を許容し、amount < 0 も満たします。balance() は expires_at > now で除外するので影響ありません。

インターフェース面では、RefundIntoExpiredBucket は refund 専用エラーとして残し、void 側は成功レスポンスに「失効により復元されなかった額」を含める形が素直だと思います。運用上の注意として、expired_amount / 未使用残高レポートが void のたびに動く点は共有が必要です。

3. ドキュメントの更新

docs/domain.md の返金/取消の節に「元バケットが既に失効済みなら復元は即時利用不可(失効価値は復活しない)」という記述が残っており、本 PR のハード拒否と食い違っています。2 を入れると refund と void で挙動が分かれるので、両方を明記する形に書き換えてください。

docs/api.md も、新しい 422 が refund セクションにしか記載されておらず、void セクションは「レスポンスは常に refund 形式」のままです。2 を入れるなら void のレスポンスに失効分のフィールドが増えるため、あわせて更新をお願いします。

ご指摘ありがとうございます。3点とも対応しました。

1. 復元プラン作成時の SELECT がバケット行をロックしていない件

FOR UPDATE OF tb を追加しました(8b63093)。expire_due() は SKIP LOCKED でスイープするため、ロックなしの読み取りだと後続の UPDATE までの間にスイープが割り込めます。別コネクションでバケット行を掴んだまま refund() を呼び、その FOR UPDATE OF tb を含む SELECT が pg_stat_activity 上でロック待ちになることを直接確認するテストを追加しています。

2. void も同じ復元処理を通るため、失効済みバケットがあると技術的な取消が恒久的に不可能になる件

復元先バケットの失効時ポリシーを分岐させました(591d6ac)。返金は従来どおり拒否(RefundIntoExpiredBucket)ですが、取消は復元せずに reversal(+)と expiry(−)を同時に記帳し、失効益として計上します。設計判断は2点です:

• topup_buckets 自体は更新しません。ペアの記帳が相殺するので、バケットの remaining_amount/status は失効のまま変わらず、次回スイープが同じ価値を二重に失効させることもありません
• 冪等リプレイの再構成のため、この expiry ポスティングは取消の transaction_id を持たせています(スイープ由来の失効は従来どおり transaction_id が NULL)

レスポンスは expired(バケット別の失効内訳)を追加しました。amount = Σrestorations + Σexpired です:

{ "transaction_id": "…", "payment_id": "…", "amount": 500, "balance": 200,
  "restorations": [ { "bucket_id": "…", "amount": 200 } ],
  "expired":      [ { "bucket_id": "…", "amount": 300 } ], "replayed": false }

3. ドキュメントの記述が挙動と不一致な件

docs/domain.md の返金/取消の記述(857ee62, 591d6ac)と docs/api.md の void セクション(591d6ac)を現行挙動に合わせました。

検証

cargo fmt --all --check / cargo clippy --workspace --all-targets --locked -- -D warnings / cargo test --workspace --locked はいずれも green です。追加テストは6本(melon-db の money テストが24→30)。

なお web/lib/types.ts の RefundResp がサーバ応答から乖離していた(payment_txn_id → 実際は payment_id、restorations が欠落)のに気付いたので同期しました(13e4870)。取消完了時の画面表示も、一部が失効していて全額は復元されなかった場合にそれが伝わるよう文言を直しています(e2dd353)。どちらも本 PR の変更が引き金なので同じ差分に含めています。

更新分（8b63093〜e2dd353）を確認しました。前回の 2（void の恒久ブロック）と 3（ドキュメント）は解消済みです。1 だけ、まだ競合が閉じていないので再度コメントします。

1. FOR UPDATE OF tb だけでは競合が閉じていません（要修正）

失効判定が expires_at と呼び出し側の now の比較のみで、topup_buckets.status を見ていません。

let expired = to_jiff(r.try_get("expires_at")?) <= now;

expires_at は不変なので、ロックを取って読み直しても判定結果は変わらず、追加したロックが判定に対して効果を持ちません。

問題になるのは次の順序です。

1. refund が now（有効期限のわずかに手前）で開始し、FOR UPDATE OF tb で待たされる
2. expire_due() が当該バケットを失効させる（expiry ポスティング記帳、remaining_amount = 0、status = 'expired'）
3. refund が再開し、expires_at > now なので「有効」と判定してプランに載せる
4. UPDATE topup_buckets SET remaining_amount = remaining_amount + $1, status = 'active' が走る

結果、expires_at が過去の expired バケットが active として復活します。balance() は expires_at > now で除外するので利用者には見えず、API は復元成功を返し、次のスイープが同じ価値を再び失効益に計上します — 本 PR が閉じようとしていた二重計上そのものです。

tb.status も SELECT して、判定に含めてください。

SELECT le.bucket_id, le.amount, tb.expires_at, tb.status FROM ledger_entries le
  JOIN topup_buckets tb ON tb.id = le.bucket_id
 WHERE le.transaction_id = $1 AND le.kind = 'payment'
 ORDER BY le.seq
   FOR UPDATE OF tb

let expired = to_jiff(r.try_get("expires_at")?) <= now
    || r.try_get::<String, _>("status")? == "expired";

これで FOR UPDATE OF tb が本来の役目を果たし、スイープ後に再開した refund は Reject、void は Forfeit に正しく分岐します。テストとしては、スイープを挟んだうえで「期限直前の now」を渡す refund/void を追加すると回帰を押さえられます。

2. docs/api.md のレスポンス例

/v1/refunds の例に expired がありません。サーバーは refund でも常に（[] として）シリアライズするので、void セクションと同様に追記をお願いします。

軽微

docs/domain.md の「バケットの status / remaining_amount は失効のまま」という記述は、厳密にはスイープ後にのみ成り立ちます。スイープ前に void した場合、バケットはまだ active で残額も残っており、その残額は後続のスイープが失効させます（この挙動自体は正しく、二重計上にもなりません）。文言だけ調整いただけると誤解がないと思います。

前回指摘の 2・3 については、実装・会計面ともに問題ありませんでした。

cargo check --all-targets と cargo clippy --all-targets はクリーンでした。

指摘いただいた 3 点はすべて修正しました。

1. FOR UPDATE OF tb が判定に対して無効だった件

ご指摘のとおりです。expires_at は生成後に一度も UPDATE されない不変列で、判定に使っていたのはこれだけでした。tb.status を SELECT と判定式の両方に加えたことで、ロックが実際に意味を持つようになりました（status == "expired" を見る形にしています。status != "active" にすると pay() が使い切ったバケットに付ける exhausted まで拒否してしまうため、そこは区別しています）。

再現テストは実スレッド競合を使わず、「先にスイープを走らせ、後からスイープ前の now で返金/取消を呼ぶ」形で決定論的に組んでいます（refund_rejects_a_bucket_swept_after_it_started / void_forfeits_a_bucket_swept_after_it_started）。

2. docs/api.md の返金応答例に expired がない件

RefundResp.expired は skip_serializing_if が付いていないため常に返るので、"expired": [] を追記しました。

3. docs/domain.md の取消後のバケット状態の記述

失効枝は topup_buckets を一切 UPDATE しないため、スイープ前に取消した場合は active のまま残額も残ります。記述をスイープ前後で分けました。

ついでに見つかった点（ご指摘とは別に）

diff 全体を見直す過程で、以下 2 点の不正確な記述も見つけたので直しました。実装（1 の修正）自体に変更はありません。

• 「失効バケットへの返金を拒否する理由」として書いていた「次のスイープが二重計上する」は、算術的に成立しないことに気づきました（topup 1000 / pay 400 / sweep 600 の例で計算すると、仮に返金が通っても失効益は元チャージ額と同じ 1000 にしかならず、二重にはなりません）。本当の理由は「復元しても balance() が除外するので利用者は使えない」ことと「スイープ済み行を active に戻すと自身の expiry ポスティングと矛盾する」ことなので、コメントと docs をそちらに差し替えました。
• 同じ箇所で「元決済のバケットが失効していれば返金は全体が拒否される」と書いていましたが、復元は逆消費順で行われるため、失効前のバケットだけで賄える部分返金は実際には成立します。範囲を正確にしました。

/v1/payments/refundable について

このエンドポイントは取引レベルの集計のみで、バケットの失効を見ていません。そのため失効済みバケットの支払いも一覧に残り続け、返金を実行すると 422 になります。今回のスコープでは挙動は変えず、現行の契約としてドキュメントとテストで明文化しました。復元可能額を返すよう一覧側を拡張する話は、スコープが変わるので別で対応することを推奨します。

各コミットは単体で clippy・全テストが通ることを確認済みです。

docs/domain.md の void 段落が、同じ PR のテストと矛盾しています

現在の記述はバケット行の事後状態を 2 分岐で説明しています。

（バケット行は一切更新しない。スイープ済みなら expired/残額 0 のまま、未スイープなら active で残額も残り、その残額は後続のスイープが回収する）

このうち後半の「未スイープなら active で残額も残り、その残額は後続のスイープが回収する」が、支払いがバケットを使い切っていた場合に当てはまりません。使い切られたバケットは ops.rs の status = CASE WHEN remaining_amount - $1 = 0 THEN 'exhausted' ELSE status END により exhausted / 残額 0 になり、expire_due() は status = 'active' AND remaining_amount > 0 しか拾わないため、期限を過ぎても永久にスイープされません。回収される残額も存在しません。

本 PR で追加された void_splits_restored_and_expired_buckets がまさにこの構成で、コード側のコメントは正しく説明しているうえ swept.expired_buckets == 0 をアサートしています。つまり、すぐ隣に置かれたテストとドキュメントが食い違っている状態です。

実際には 3 分岐になるので、次のような書き換えを提案します。

(バケット行は一切更新しない。スイープ済みなら `expired`/残額 0 のまま、支払いが使い切っていれば
`exhausted`/残額 0 のままでスイープの対象にもならず、残額があってまだ未スイープなら `active` の
ままで、その残額は後続のスイープが回収する)

なお、この 3 つのいずれの経路でも void 自身が expiry(−) を記帳するため失効益の計上は 1 回だけで、挙動そのものは正しいです。ドキュメントの記述のみの問題です。

該当箇所を 3 分岐に修正しました:

(バケット行は一切更新しない。スイープ済みなら expired/残額 0 のまま、使い切られていれば exhausted/残額 0 のままでスイープの対象にもならず、残額が残っていてまだ未スイープなら active のままで、その残額は後続のスイープが回収する)

ご提案から変えたのは「支払いが使い切っていれば」→「使い切られていれば」の 1 語のみです。バケットは当該支払い以外の支払いでも残額 0 になりうるため、原因を特定しない表現にし、status の 3 値(expired / exhausted / active)で排他かつ網羅に場合分けしています。

同じ段落・同じ節で、上記と同種の不正確な記述が他に見つかったため、あわせて修正しました:

• ops.rs の void 分岐のコメントが「バケット行は無変更で再スイープ対象は残らない」と書いていましたが、これは同じ PR の void_forfeits_expired_value_before_any_sweep が示す「未消費の残額は後続のスイープが回収する」と矛盾していました。「この void が失効計上した分は二度とスイープされない」という趣旨に直しています。
• audit_refund の doc コメントが「復元する」とだけ書いていましたが、本 PR 以降 void は失効済みバケットへは復元せず失効計上するため、関数本体(expired = ... の集計)と doc が食い違っていました。
• domain.md の「同じ形の expiry ポスティング」は、同ファイル内の「スイープ由来は transaction_id が NULL、取消由来は当該取消の transaction_id を持つ」という記述と矛盾していたため、transaction_id の違いを明記する形に直しました。
• void() の doc と api.md の「取消は成立します」が、先行する部分返金がある場合や IDEMPOTENCY_CONFLICT 時に成立しない「全額」「無条件」という言い方をしていたため、それぞれ「残返金可能額」「失効を理由には拒否されない」に正確化しました。
• api.md の返金エンドポイント側にも同じ「全額」表現が残っていたため(amount 省略時の挙動)、「残額全部」に揃えました。
• merchant-guide.md(加盟店向けガイド)の返金セクションが「元のチャージが失効していれば、戻した残高も失効した状態になり使えない」と書いていましたが、本 PR で失効済みバケットへの返金は復元前に 422 で拒否されるようになった(REFUND_INTO_EXPIRED_BUCKET)ため、「戻すが使えない」状態自体が発生しなくなっています。「返金はできません」に修正し、端末のエラーメッセージ文言に合わせたエラー表の行も追加しました。

3 分岐はいずれも既存テストで固定されています:

挙動(コード)の変更はありません。ドキュメントとコメントの修正のみです。

なお、上記と同じ前提(失効済みバケットへの返金が「成立するが使えない残高になる」)が terms.md / merchant-terms.md(利用規約)にも残っています。こちらは公開済みの規約文書のため、本 PR では変更していません。規約の改定は別途ご判断いただくのがよいと考えています。

更新分（7c293c4 / a00aabc / 63f7eae）を確認しました。前回指摘した docs/domain.md の void 段落は 3 分岐（expired/残額 0 → exhausted/残額 0 でスイープ対象外 → active で残額あり、後続スイープが回収）に書き換わり、expire_due() のフィルタと一致し、void_splits_restored_and_expired_buckets とも矛盾しなくなりました。

今回追加・修正された記述はコードと突き合わせて確認し、いずれも正確でした。
残りは文言のみ 3 点です。コードの欠陥は見つかりませんでした。

1. web/content/merchant-guide.md — 「現金で精算」の案内が危険

失効したチャージについて現金精算を案内していますが、取消(void)自体は成功する（価値は失効益に計上され、加盟店には引き落としが立つ）ことに触れていません。案内どおり現金を渡したうえで取消も実行すると、加盟店負担で顧客に二重に支払うことになります。「取消を行うか、現金で精算するかのいずれか一方」であることを明記してください。

2. web/content/merchant-guide.md — 「元のチャージがすでに失効していた場合、返金はできません」は言い過ぎ

まだ有効なバケットで賄える範囲の部分返金は成立します（docs/api.md にも「失効前のバケットだけで賄える部分返金は成立します」とあります）。「復元先が失効済みバケットに及ぶ場合に拒否される」という趣旨に直すのが正確です。

3. void の確認ダイアログが「全額」のまま（diff 外）

web/app/(ja)/merchant/transactions/page.tsx と web/app/(ja)/admin/accounts/page.tsx の取消確認ダイアログは「全額」と表示しています。本 PR はドキュメント側を「残返金可能額」に直しましたが、UI は追随していません。部分返金済みの支払いを取消すと、確認した額と実際に記帳される額が食い違います。今回の diff 外ですが、この PR が正そうとしたのと同じ記述なので、あわせて直すのが自然だと思います。

ご指摘の3点、および同種の記述が他に残っていた箇所をあわせて修正しました。

1. merchant-guide.md — 「現金で精算」の案内が失効益との二重払いに触れていなかった件

取消し(void)自体は成立し、加盟店の精算残高から取消額の全額が差し引かれることを明記したうえで、「取消しを行うか、現金で精算するかのいずれか一方」であることを追記しました(79d9073)。あわせて、取消しは一部だけを戻すことができない(残返金可能額をまとめて戻す操作)点も明記しています。部分返品のケースで、取消しと現金精算の選び方に迷わないようにする意図です。

2. merchant-guide.md — 「元のチャージがすでに失効していた場合、返金はできません」が言い過ぎだった件

「戻し先のチャージ分が失効している場合、その分は返金できません。まだ失効していないチャージ分だけで賄える金額であれば、その範囲で返金できます」に修正しました(79d9073)。8章のエラー表も、端末の実際の表示文言(1・2列目)はそのままに、対応欄を「金額を減らせば返金可、残りは取消しか現金精算のどちらか一方」に直しています。

3. 取消の確認ダイアログが「全額」のままだった件(diff外)

admin/accounts/page.tsx は返金可能な支払い一覧から refundable を保持しているため、確認ダイアログに実際の取消額を表示するようにしました(この支払いの返金可能額 ¥800 を取消しますか? のように金額を出します、4778799)。merchant/transactions/page.tsx の取引一覧は個々の支払いの返金可能額を保持していない(MerchantTxn に該当フィールドがなく、追加するには一覧APIの拡張が必要でスコープが変わります)ため、こちらは文言のみ「残り全額(返金済みを除く)」に正確化しています。

あわせて直した箇所(同種の記述)

上記3点と同じ「全額 → 実際は残返金可能額」の誤りが、diffの外にも残っていたため、あわせて修正しました。

• 返金プロンプトの既定値の説明(admin/accounts/page.tsx・merchant/transactions/page.tsx の refund 関数、54589e4) — 「空欄で全額」ではなく、実際は残返金可能額が入ります。
• docs/operations.md の端末返金フローの説明(既定=全額 → 既定=返金可能額、ff71a51) — 端末は p.refundable を初期値・上限にしています。
• merchant-guide.md 6章冒頭の「(返金・取消しは)端末からも、加盟店ポータルからもできます」(79d9073) — 取消しは加盟店ポータル専用です(端末のタブは支払い/チャージ/残高/返金の4つで、取消し操作はありません)。返金と取消しを分けて記載し直しました。
• merchant-guide.md 3章の端末返金フローの「返金額(全額または一部)」(79d9073) — 同じ理由で「返金可能額の全部または一部」に直しました。

検証

Rustコードへの差分はありません。今回の差分に対しては cd web && npm run typecheck / npm run build がいずれも green です。4コミットは各単体で git rebase --exec により build・typecheck が通ることを確認済みです。

更新分（79d9073 / 4778799 / 54589e4 / ff71a51）を確認しました。前回の 3 点は対応されていますが、新しいバグが 1 件混入しているので先に挙げます。

1. 返金額の入力で桁区切りが誤解釈されます（要修正・今回の変更で混入）

web/app/(ja)/admin/accounts/page.tsx の prompt 表示を ${refundable} から ${yen(refundable)} に変えたことで ¥1,000 のようなカンマ区切りが出るようになりましたが、入力の解釈は parseInt のままです。

const input = prompt(
  `返金額(円)。空欄で返金可能額 ${yen(refundable)} を返金します。`,
);
if (input === null) return;
const amount = input.trim() === "" ? null : parseInt(input, 10);

表示された額をそのまま打ち返すと parseInt("1,000", 10) → 1 円が返金されます。amount > 0 は満たすのでバリデーションにも掛からず、そのまま記帳されます。変更前は生の数値表示だったため打ち返しが成立していました。

入力側で input.replace(/[,¥\s]/g, "") してからパースするか、表示を区切りなしに戻してください。

2. web/content/merchant-guide.md — 「一部返品のときは現金など他の方法での精算をご検討ください」が誤り

部分返金はサポートされており、同じファイルの手順 3・エラー表・端末 UI のいずれとも矛盾します。この案内に従うと顧客が二重に受け取ることになります。

3. docs/operations.md — 直前行と矛盾する記述が残っています

今回修正された行（#refAmt に p.refundable がプリフィルされ、送信時にその額でクランプされる）は正しくなりました。ただしその下の「テンキー入力・Enter で実行」という記述は、直上で説明している返金額には当てはまりません（op === "refund" ではテンキーは非表示、Enter はオーバーレイ側の分岐で消費されます）。

ご指摘の3点を修正しました。

1. 返金額プロンプトの桁区切りが誤解釈される件

表示だけ yen() に変えて入力の解釈を parseInt のままにしていたため、表示どおり打ち返すと parseInt("1,000", 10) → 1 が送信されていました。

ご提案のうち表示を区切りなしに戻す方を採りました(97dcd59)。

返金額(円)。空欄で返金可能額 800 円を返金します。

同じ画面の取消確認ダイアログは入力を伴わない確認なので yen() のままにしています。prompt() で金額を受けるのはこの返金と加盟店ポータルの返金の2箇所のみで、他の金額入力はいずれも <input type="number"> のため桁区切りは入りません。

2. merchant-guide.md の「一部返品のときは現金など…」

削除し、一部だけ戻すときは返金を使う旨に直しました(aed5bb0)。同ファイルの手順3・8章のエラー表とも整合します。

当初「戻し先が失効していて返金できないときは、上の2つ(取消しか現金)のどちらか一方で対応」という書き方をしていましたが、void() は返金額を指定できず残返金可能額の全額しか戻せないため、一部だけを戻したい場面で取消しを選ぶと意図した範囲を超えて全額を巻き戻してしまいます。取消しを外し、現金精算のみに絞りました。

3. docs/operations.md のテンキー・Enter の記述

返金額は一覧から支払いを選んだ後のシート内の入力欄で、terminal.html ではテンキーは op が pay / topup のときだけ表示され、オーバーレイ表示中の Enter は完了シートを閉じる分岐に入るため実行には使われません。支払い/チャージの金額入力と返金額を分けて書き直しました(27b286b)。

最初の書き方だと「返金額はシート内の入力欄に直接入力」のすぐ後に「Enter は実行ボタン相当」と続けており、返金額入力中も Enter で実行できるように読めました。実際には openOverlay() はカードをかざす前(最初の実行ボタン押下時)に呼ばれ、返金額入力欄はこのオーバーレイの中にあるため、handleKeydown() のオーバーレイ分岐に入り Enter は無効です。「カード読み取り開始前」の実行ボタンに限定し、返金額入力中は効かないことを明記しました。

検証

Rust コードへの差分はありません。cd web && npm run typecheck / npm run build はいずれも green で、3コミットとも単体で通ることを git rebase --exec で確認しています。